SOLR-17806: Migrate DirectUpdateHandler2 metrics to OTEL #3417

mlbiscoc · 2025-07-03T18:02:55Z

https://issues.apache.org/jira/browse/SOLR-17806

Migrate DirectUpdateHandler2 metrics and remove Dropwizard initializations.

# HELP solr_metrics_core_update_auto_commits_total Total number of auto commits
# TYPE solr_metrics_core_update_auto_commits_total counter
solr_metrics_core_update_auto_commits_total{category="UPDATE",collection="demo",core="demo_shard1_replica_n1",otel_scope_name="org.apache.solr",replica="replica_n1",shard="shard1",type="auto_commits"} 0.0
solr_metrics_core_update_auto_commits_total{category="UPDATE",collection="demo",core="demo_shard1_replica_n1",otel_scope_name="org.apache.solr",replica="replica_n1",shard="shard1",type="soft_auto_commits"} 0.0
# HELP solr_metrics_core_update_commit_stats Metrics around commits
# TYPE solr_metrics_core_update_commit_stats gauge
solr_metrics_core_update_commit_stats{category="UPDATE",collection="demo",core="demo_shard1_replica_n1",otel_scope_name="org.apache.solr",replica="replica_n1",shard="shard1",type="auto_commit_max_time"} 15000.0
solr_metrics_core_update_commit_stats{category="UPDATE",collection="demo",core="demo_shard1_replica_n1",otel_scope_name="org.apache.solr",replica="replica_n1",shard="shard1",type="soft_auto_commit_max_time"} 3000.0
# HELP solr_metrics_core_update_errors_total Total number of update errors
# TYPE solr_metrics_core_update_errors_total counter
solr_metrics_core_update_errors_total{category="UPDATE",collection="demo",core="demo_shard1_replica_n1",otel_scope_name="org.apache.solr",replica="replica_n1",shard="shard1"} 0.0
# HELP solr_metrics_core_update_maintenance_operations_total Total number of maintenance operations
# TYPE solr_metrics_core_update_maintenance_operations_total counter
solr_metrics_core_update_maintenance_operations_total{category="UPDATE",collection="demo",core="demo_shard1_replica_n1",operation="rollback",otel_scope_name="org.apache.solr",replica="replica_n1",shard="shard1"} 0.0
solr_metrics_core_update_maintenance_operations_total{category="UPDATE",collection="demo",core="demo_shard1_replica_n1",operation="split",otel_scope_name="org.apache.solr",replica="replica_n1",shard="shard1"} 0.0
# HELP solr_metrics_core_update_operations_cumulative Cumulative number of update commands processed. Metric can go down from rollback command
# TYPE solr_metrics_core_update_operations_cumulative gauge
solr_metrics_core_update_operations_cumulative{category="UPDATE",collection="demo",core="demo_shard1_replica_n1",operation="adds",otel_scope_name="org.apache.solr",replica="replica_n1",shard="shard1"} 0.0
solr_metrics_core_update_operations_cumulative{category="UPDATE",collection="demo",core="demo_shard1_replica_n1",operation="commits",otel_scope_name="org.apache.solr",replica="replica_n1",shard="shard1"} 0.0
solr_metrics_core_update_operations_cumulative{category="UPDATE",collection="demo",core="demo_shard1_replica_n1",operation="deletes_by_id",otel_scope_name="org.apache.solr",replica="replica_n1",shard="shard1"} 0.0
solr_metrics_core_update_operations_cumulative{category="UPDATE",collection="demo",core="demo_shard1_replica_n1",operation="deletes_by_query",otel_scope_name="org.apache.solr",replica="replica_n1",shard="shard1"} 0.0
solr_metrics_core_update_operations_cumulative{category="UPDATE",collection="demo",core="demo_shard1_replica_n1",operation="expunge_deletes",otel_scope_name="org.apache.solr",replica="replica_n1",shard="shard1"} 0.0
solr_metrics_core_update_operations_cumulative{category="UPDATE",collection="demo",core="demo_shard1_replica_n1",operation="merges",otel_scope_name="org.apache.solr",replica="replica_n1",shard="shard1"} 0.0
solr_metrics_core_update_operations_cumulative{category="UPDATE",collection="demo",core="demo_shard1_replica_n1",operation="optimize",otel_scope_name="org.apache.solr",replica="replica_n1",shard="shard1"} 0.0
# HELP solr_metrics_core_update_pending_operations Operations pending commit. Values get reset after commit
# TYPE solr_metrics_core_update_pending_operations gauge
solr_metrics_core_update_pending_operations{category="UPDATE",collection="demo",core="demo_shard1_replica_n1",operation="adds",otel_scope_name="org.apache.solr",replica="replica_n1",shard="shard1"} 0.0
solr_metrics_core_update_pending_operations{category="UPDATE",collection="demo",core="demo_shard1_replica_n1",operation="deletes_by_id",otel_scope_name="org.apache.solr",replica="replica_n1",shard="shard1"} 0.0
solr_metrics_core_update_pending_operations{category="UPDATE",collection="demo",core="demo_shard1_replica_n1",operation="deletes_by_query",otel_scope_name="org.apache.solr",replica="replica_n1",shard="shard1"} 0.0
solr_metrics_core_update_pending_operations{category="UPDATE",collection="demo",core="demo_shard1_replica_n1",operation="docs_pending",otel_scope_name="org.apache.solr",replica="replica_n1",shard="shard1"} 0.0

dsmiley

Dealing with rollbacks is annoying... it's tempting to ignore them but I suppose we should not.

I would like to explore here monotinically increasing numbers that can be subtracted from each other to determine "pending" if needed. Maybe the prometheus endpoint could have a convenience to compute pending as well.

dsmiley · 2025-07-04T03:07:34Z

solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java

-  Meter numErrorsCumulative;
+
+  // Cumulative commands
+  AttributedLongUpDownCounter deleteByQueryCommandsCumulative;


why are these "cumulative" things having a type that includes "Down"; wouldn't they never go down?

They go down from a rollback command.

addCommandsCumulative.add(-addCommands.sumThenReset()); deleteByIdCommandsCumulative.add(-deleteByIdCommands.sumThenReset()); deleteByQueryCommandsCumulative.add(-deleteByQueryCommands.sumThenReset());

Although the maintenance operations below only go up, so we can move that to an normal counter actually.

dsmiley · 2025-07-04T04:47:13Z

solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java

+    updateStats =
+        solrMetricsContext.observableLongGauge(
+            "solr_metrics_core_update_pending_operations",
+            "Operations pending commit. Values get reset after commit",


I wonder if it would be better to instead track "submitted", as separate from "committed" operations? Much like the started and completed merges -- the PR Kevin Liang is working on -- #3416

Hmmm maybe that is better. Maybe I'll reduce this to only docs_pending for this gauge instead then we'll move to submitted and committed increasing counters. I'll refactor this around and see how it looks.

I went ahead an created submitted and committed and removed the pending metric. If you think this implementation is good, I'll fix up the tests accordingly for the metric. Here is what it looks like. WDTY?

# HELP solr_core_update_committed_ops_total Total number of committed update operations # TYPE solr_core_update_committed_ops_total counter solr_core_update_committed_ops_total{category="UPDATE",collection="gettingstarted",core="gettingstarted_shard1_replica_n1",ops="adds",otel_scope_name="org.apache.solr",replica="replica_n1",shard="shard1"} 1.0 solr_core_update_committed_ops_total{category="UPDATE",collection="gettingstarted",core="gettingstarted_shard1_replica_n1",ops="deletes_by_id",otel_scope_name="org.apache.solr",replica="replica_n1",shard="shard1"} 1.0 solr_core_update_committed_ops_total{category="UPDATE",collection="gettingstarted",core="gettingstarted_shard1_replica_n1",ops="deletes_by_query",otel_scope_name="org.apache.solr",replica="replica_n1",shard="shard1"} 1.0 # HELP solr_core_update_submitted_ops_total Total number of submitted update operations # TYPE solr_core_update_submitted_ops_total counter solr_core_update_submitted_ops_total{category="UPDATE",collection="gettingstarted",core="gettingstarted_shard1_replica_n1",ops="adds",otel_scope_name="org.apache.solr",replica="replica_n1",shard="shard1"} 1.0 solr_core_update_submitted_ops_total{category="UPDATE",collection="gettingstarted",core="gettingstarted_shard1_replica_n1",ops="deletes_by_id",otel_scope_name="org.apache.solr",replica="replica_n1",shard="shard1"} 1.0 solr_core_update_submitted_ops_total{category="UPDATE",collection="gettingstarted",core="gettingstarted_shard1_replica_n1",ops="deletes_by_query",otel_scope_name="org.apache.solr",replica="replica_n1",shard="shard1"} 1.0

dsmiley · 2025-07-04T04:57:35Z

solr/core/src/test/org/apache/solr/cloud/TestTlogReplica.java

+              .getSolrMetricsContext()
+              .getMetricManager()
+              .getPrometheusMetricReader(solrCore.getCoreMetricManager().getRegistryName());
+      var actual =


this statement is lacking some elegance. If this pattern repeats itself; we should seek an improvement

Ok so I overhauled the util method a bit for getting metrics in SolrMetricTestUtils. Maybe this is a bit better? If so, I will adopt this paradigm on the other SolrCore PR I did and other PRs going forward.

solr/core/src/test/org/apache/solr/metrics/SolrMetricTestUtils.java

dsmiley · 2025-07-04T05:05:45Z

solr/core/src/test/org/apache/solr/update/DirectUpdateHandlerTest.java

-    assertEquals("new adds", 2, newAdds - adds);
-    assertEquals("new cumulative adds", 2, newCumulativeAdds - cumulativeAdds);
+    assertEquals(
+        "Did not have the expected number of pending adds",


as you went through the effort of writng these messages -- fine. But I wouldn't waste your time; it's optional.

dsmiley · 2025-07-04T05:09:24Z

solr/core/src/test/org/apache/solr/update/DirectUpdateHandlerTest.java

+        "Did not have the expected number of cumulative deletes_by_id",
+        1,
+        getGaugeOpDatapoint(
+                reader, "solr_metrics_core_update_operations_cumulative", "deletes_by_id")


just a quick observation -- the "metrics" part of these names seems redundant; no?
The "solr" part is I know needed to differentiate the JVM & Jetty.

Yeah but mostly kept them as it was the way the prometheus exporter prefixed all it's metrics. Definitely no need to keep them if we don't want. I removed them in the latest commit but will remove it from the other metrics in a separate PR.

dsmiley · 2025-07-04T05:11:45Z

solr/webapp/web/js/angular/controllers/plugins.js

@@ -15,6 +15,8 @@
 limitations under the License.
 */

+//NOCOMMIT: This plugin seems tied to the Admin UIs plugin management but is tied to dropwizard metrics failing some tests.
+// This needs to change how it gets these metrics or we need to make a shim to the /admin/plugins handler for this to support it


Looking at PluginInfoHandler, I suppose we can continue to support that.

dsmiley

minor: I wonder if "ops" would be sufficiently clear instead of "operations"? After all, we have "stats", not "statistics".

dsmiley · 2025-07-10T00:40:35Z

solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java

@@ -262,27 +262,29 @@ public void initializeMetrics(
            baseCommandsMetric,
            baseAttributes.get().put(OPERATION_ATTR, "deletes_by_query").build());

+    var baseCommitMetric =


Why is this a "base"; why do you refer to it in the metrics for commands & merges below other than "commits"?

solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java

dsmiley

your improvements are a net improvement IMO; thanks for considering my feedback

dsmiley · 2025-07-18T23:10:15Z

solr/core/src/test/org/apache/solr/cloud/TestTlogReplica.java

+              "solr_core_update_docs_pending_commit",
+              Labels.builder()
+                  .label("category", "UPDATE")
+                  .label("operation", "docs_pending")


that'd be "ops" now?

I was doing a hasSameValues instead of equals which is why it was still passing. Should be fixed now.

dsmiley · 2025-07-18T23:12:13Z

solr/core/src/test/org/apache/solr/cloud/TestTlogReplica.java

+                  .label("category", "UPDATE")
+                  .label("operation", "docs_pending")
+                  .build(),
+              true);


nit: booleans like here are inscrutable when reviewing code. Not a big deal though

dsmiley · 2025-07-18T23:14:22Z

solr/core/src/test/org/apache/solr/update/DirectUpdateHandlerTest.java

-        getGaugeOpDatapoint(reader, "solr_core_update_operations_cumulative", "adds").getValue(),
+        getGaugeOpDatapoint(core, "solr_core_update_cumulative_ops", "adds").getValue(),


passing core instead of reader is clearer; thanks

mlbiscoc added 6 commits June 30, 2025 17:39

Migrate direct update handler

8ed6c85

Fix up metrics

58c6eec

UNCOMMIT ME BEFORE PUSHING

44265eb

Revert requestHandlerBase

7752dfd

Fix broken tests

1e43bef

Cleanup

fc0a690

github-actions bot added admin-ui tests cat:cloud cat:index cat:metrics labels Jul 3, 2025

dsmiley reviewed Jul 4, 2025

View reviewed changes

Move commit ops to normal counter

6d2e761

dsmiley reviewed Jul 10, 2025

View reviewed changes

mlbiscoc added 2 commits July 18, 2025 17:01

Add new submitted and committed metrics

eeac8f8

Move methods around

d08dcb3

dsmiley reviewed Jul 18, 2025

View reviewed changes

Fix tests for submitted and committed

5f344f5

		getGaugeOpDatapoint(reader, "solr_core_update_operations_cumulative", "adds").getValue(),
		getGaugeOpDatapoint(core, "solr_core_update_cumulative_ops", "adds").getValue(),

SOLR-17806: Migrate DirectUpdateHandler2 metrics to OTEL #3417

Are you sure you want to change the base?

SOLR-17806: Migrate DirectUpdateHandler2 metrics to OTEL #3417

Uh oh!

Conversation

mlbiscoc commented Jul 3, 2025

Uh oh!

dsmiley left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dsmiley left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dsmiley left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!